Don't use z.infer to fix closure compiler issues#1221
Don't use z.infer to fix closure compiler issues#1221jiahaog wants to merge 1 commit intogoogle:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the A2UI schema definitions across versions v0.8 and v0.9 by replacing implicit Zod inference with explicit TypeScript interfaces and type aliases. Zod schemas are now explicitly typed using these interfaces to improve type safety and clarity. Feedback focuses on removing redundant property re-declarations in extended interfaces within the v0.8 types and restoring a lost documentation comment in the v0.9 schema.
jiahaog
force-pushed
the
zinfer
branch
5 times, most recently
from
1a6f8d9 to
1f9b9d4
Compare
jiahaog
changed the title
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the A2UI schema definitions across versions v0.8 and v0.9 to use explicit TypeScript interfaces alongside Zod schemas, rather than relying solely on z.infer. This change improves type safety and provides clearer documentation for the message structures. Additionally, it updates component APIs and tests to reflect these schema changes. The reviewer noted that some new type aliases for message lists are redundant and suggested using array types directly for better idiomatic consistency.
jiahaog
force-pushed
the
zinfer
branch
2 times, most recently
from
a5d8071 to
c30f8dc
Compare
jacobsimionato
left a comment
jacobsimionato
left a comment
There was a problem hiding this comment.
I think I like this. It seems easier to understand and for an agent to follow than doing a pattern like z.infer extends etc.
Let's chat with @sugoi-yuzuru though, who has been pursuing an alternative approach within the google repo - see CL/899744784
- The Closure Compiler renames properties and variables to minimize code size. -`z.infer` Relies on Type Names and works by inspecting the shape of the schema definition at compile time to create a TypeScript type. However, the runtime validation logic within Zod likely relies on the property names defined in the schema. The two are mutually exclusive. To fix this, introduce some duplication of the types. The duplication should be mostly okay as if they are changed to not match, typescript will fail the typecheck except if there are optional fields introduced in the schema. ### API changes • renderers/web_core/src/v0_9/schema/common-types.ts : Added ChecksSchema as a new top-level export (to avoid using .shape.checks on strictly typed schemas). • renderers/web_core/src/v0_9/schema/client-to-server.ts : The interface A2uiValidationError and its schema A2uiValidationErrorSchema were renamed to A2uiValidationErrorData and A2uiValidationErrorDataSchema respectively (to resolve the duplicate export clash with errors.ts in index.ts ). Related: b/502243995 Fixes b/495161036 Fixes google#896
|
@sugoi-yuzuru any thoughts about this? |
2 participants
Description
z.inferRelies on Type Names and works by inspecting the shape of the schema definition at compile time to create a TypeScript type. However, the runtime validation logic within Zod likely relies on the property names defined in the schema.The two are mutually exclusive. To fix this, introduce some duplication of the types. The duplication should be mostly okay as if they are changed to not match, typescript will fail the typecheck except if there are optional fields introduced in the schema.
Coding agents can easily address this duplication. Also added a github actions to check for
z.infer.API changes
• renderers/web_core/src/v0_9/schema/common-types.ts : Added ChecksSchema as a new top-level
export (to avoid using .shape.checks on strictly typed schemas).
• renderers/web_core/src/v0_9/schema/client-to-server.ts : The interface A2uiValidationError and
its schema A2uiValidationErrorSchema were renamed to A2uiValidationErrorData and
A2uiValidationErrorDataSchema respectively (to resolve the duplicate export clash with errors.ts
in index.ts ).
Related: b/502243995
Fixes b/495161036
Fixes #896
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.